Skip to content

bridge: add in-memory /logs endpoint to broker bridge#137

Merged
benvinegar merged 2 commits into
mainfrom
bridge/logs-endpoint
Feb 23, 2026
Merged

bridge: add in-memory /logs endpoint to broker bridge#137
benvinegar merged 2 commits into
mainfrom
bridge/logs-endpoint

Conversation

@benvinegar

@benvinegar benvinegar commented Feb 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Add a local-only GET /logs endpoint to slack-bridge/broker-bridge.mjs that returns recent bridge logs from an in-memory ring buffer.

Changes

  • Added in-memory ring buffer for last 1000 log lines (oldest dropped on overflow).
  • Updated bridge logging path so existing logInfo/logWarn/logError output still goes to stdout/stderr and is also captured in memory.
  • Added GET /logs endpoint:
    • /logs returns all buffered lines (plain text)
    • /logs?n=100 returns last N lines
    • /logs?filter=error applies case-insensitive substring filtering
    • params can be combined (/logs?n=50&filter=poll)
  • Kept everything in-memory only (no filesystem writes), naturally reset on process restart.

Tests

  • Added integration coverage in test/broker-bridge.integration.test.mjs for:
    • successful plain-text /logs response
    • filter behavior
    • n limiting
    • invalid n (400)

Validation

  • npm run typecheck
  • npm test

Behavior impact

No change to broker send/pull/ack protocol behavior. Existing /send, /reply, /react behavior remains intact.

@greptile-apps

greptile-apps Bot commented Feb 23, 2026

Copy link
Copy Markdown

Greptile Summary

Added an in-memory ring buffer (1000 lines max) to capture bridge logs and exposed them via a local-only GET /logs endpoint with optional filtering and line limiting.

Key changes:

  • Refactored logging functions (logInfo, logWarn, logError) to capture output in logLineBuffer while still writing to stdout/stderr
  • New /logs endpoint supports ?filter= (case-insensitive substring) and ?n= (last N lines) query parameters
  • Enforces localhost-only access and applies existing rate limiting
  • Comprehensive integration test coverage for all /logs functionality

Issue found:

  • getLogLinesForResponse returns direct reference to internal buffer when no filtering/limiting applied, allowing potential mutation

Confidence Score: 4/5

  • Safe to merge after fixing the array reference issue in getLogLinesForResponse
  • Implementation is well-designed with proper security controls (localhost-only, rate-limited) and comprehensive tests. Found one logic issue where the internal log buffer reference is returned directly, which could allow mutation. Otherwise the code follows project conventions and doesn't weaken any security protections.
  • Fix the array reference issue in slack-bridge/broker-bridge.mjs:728 before merging

Important Files Changed

Filename Overview
slack-bridge/broker-bridge.mjs Added in-memory log buffer with /logs endpoint; found one reference issue in getLogLinesForResponse
test/broker-bridge.integration.test.mjs Comprehensive test coverage for new /logs endpoint with filter, limit, and error cases

Last reviewed commit: ddc1d2b

greptile-apps[bot]
greptile-apps Bot reviewed Feb 23, 2026
View reviewed changes

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread slack-bridge/broker-bridge.mjs Outdated
requestedLineCount = Math.min(parsedN, LOG_BUFFER_MAX_LINES);
}

let lines = logLineBuffer;

@greptile-apps greptile-apps Bot Feb 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when no filter and no n param, returns direct reference to logLineBuffer, allowing caller to mutate the internal buffer

Suggested change
let lines = logLineBuffer;
let lines = logLineBuffer.slice();
Prompt To Fix With AI 
This is a comment left during a code review.
Path: slack-bridge/broker-bridge.mjs
Line: 728

Comment:
when no filter and no `n` param, returns direct reference to `logLineBuffer`, allowing caller to mutate the internal buffer

```suggestion
  let lines = logLineBuffer.slice();
```

How can I resolve this? If you propose a fix, please make it concise.

@benvinegar benvinegar Feb 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — updated getLogLinesForResponse to copy the buffer before filtering/slicing (logLineBuffer.slice()), so it never returns a direct reference to internal state.

Responded by pi-coding-agent using openai/gpt-5.

@benvinegar benvinegar merged commit 59d3563 into main Feb 23, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@benvinegar